-
Notifications
You must be signed in to change notification settings - Fork 7k
Make CLI pip-installable #8772
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Make CLI pip-installable #8772
Conversation
pyproject.toml
Outdated
modal = ">=0.66.26,<0.78.0" | ||
google-cloud-aiplatform = "*" | ||
anthropic = { extras = [ "vertex" ], version = "*" } | ||
google-generativeai = "*" # To use litellm with Gemini Pro API |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right to drop these here with a TODO, they're like three different categories of things and may need some thought
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I tried to get cute with making more of these optional, but it was way too hard to figure out exactly what was truly optional. It's a painful dev loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get to test it as I intended. I think it's safe to merge though, it's weekend and no release, we can mess around and find out if there's something missing.
@OpenHands resolve the merge conflicts in pyproject.toml, and then run |
I'm on it! rbren can track my progress at all-hands.dev |
I successfully resolved the merge conflicts in the pyproject.toml file and regenerated the poetry.lock file. Here's a summary of what I did:
The PR #8772 should now be updated with these changes, and the merge conflicts have been resolved. The formatting of the pyproject.toml file also follows the project's standards, with proper alignment of comments. All requested tasks have been completed successfully. |
@OpenHands this test is newly failing
look at the diff vs main. Did we lose a dependency we need? what changed? fix it |
I'm on it! rbren can track my progress at all-hands.dev |
Summary of ChangesI fixed the failing test issue by addressing the dependency problem in the PR. Problem IdentifiedThe test
Root CauseThe PR had made the test group optional in Solution ImplementedI made two key changes:
VerificationI verified that the changes fixed the issue by running the tests again. While the specific test that was failing still has some issues with its implementation logic, the original dependency error is now resolved, and other async tests are now passing successfully. Changes PushedAll changes have been committed and pushed to the |
Looks like there are a few issues preventing this PR from being merged!
If you'd like me to help, just leave a comment, like
Feel free to include any additional details that might help me get this PR into a better state. You can manage your notification settings |
End-user friendly description of the problem this fixes or functionality this introduces.
Summarize what the PR does, explaining any non-trivial design decisions.
main
function for running the CLITesting
Link of any specific issues this addresses:
To run this PR locally, use the following command: